[clr-interp] Implement Managed Return Value support for the interpreter#127760
[clr-interp] Implement Managed Return Value support for the interpreter#127760kotlarmilos wants to merge 7 commits into
Conversation
kotlarmilos
changed the title
kotlarmilos
changed the title
|
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg |
There was a problem hiding this comment.
Pull request overview
This PR extends CoreCLR interpreter debug info so managed return values can be surfaced for interpreted calls, and broadens IL/native mapping beyond stack-empty offsets. It fits into the debugger/interpreter pipeline by teaching the interpreter to emit richer boundary data and teaching DI to decode interpreter callsites.
Changes:
- Adds interpreter-side IL/native map capacity tracking and emits per-IL-offset mappings plus
CALL_INSTRUCTIONentries for interpreter call opcodes. - Adds DI-side interpreter opcode-length decoding and synthetic stack-home lookup for interpreter return values.
- Wires
CordbJITILFrameto read interpreted return values from FP-relative dvars instead of native return registers.
Review found a blocking correctness issue: the new CALL_INSTRUCTION tagging in compiler.cpp also marks compiler-inserted helper calls (for example the helper emitted by EmitPushLdvirtftn), so GetReturnValueLiveOffset can report extra offsets for a single IL callsite and one of those offsets corresponds to the helper’s function-pointer result rather than the user call’s return value.
Show a summary per file
| File | Description |
|---|---|
src/coreclr/interpreter/compiler.h |
Adds native-map capacity tracking for interpreter-emitted debug boundaries. |
src/coreclr/interpreter/compiler.cpp |
Emits expanded IL/native mappings and interpreter callsite markers. |
src/coreclr/debug/di/rsthread.cpp |
Reads interpreted return values from synthetic FP-relative variable homes. |
src/coreclr/debug/di/rspriv.h |
Declares the new interpreter-specific DI helper. |
src/coreclr/debug/di/module.cpp |
Decodes interpreter callsite lengths and dvar offsets in DI. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 1
kotlarmilos
force-pushed
the
interp-mrv-return-value
branch
from
fe82178 to
3c1ac90
Compare
kotlarmilos
force-pushed
the
interp-mrv-return-value
branch
from
3c1ac90 to
9f2787a
Compare
kotlarmilos
force-pushed
the
interp-mrv-return-value
branch
from
9f2787a to
946174b
Compare
kotlarmilos
force-pushed
the
interp-mrv-return-value
branch
from
946174b to
9bffeb9
Compare
kotlarmilos
force-pushed
the
interp-mrv-return-value
branch
from
8afd1eb to
a1932f7
Compare
Caller uses IfFailRet which treats S_FALSE as success. With the IsInterpreted() gating in place, neither S_FALSE return path is reachable on the happy path, so propagate them as real errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kotlarmilos
force-pushed
the
interp-mrv-return-value
branch
from
a1932f7 to
23409b8
Compare
noahfalk
left a comment
noahfalk
left a comment
There was a problem hiding this comment.
@kotlarmilos - If possible lets hold off on this one until we settle what data contract we want here overall. If getting this in is blocking making progress elsewhere let me know but I'm guessing its relatively isolated. If we do wind up embedding this interpreter info into the debugger code we'll probably want to define some new IDacDbiInterface APIs and put the logic that understands interpreter disassembly in the DAC instead.
Rework interpreter managed-return-value (MRV) support to use the CALL_RETURN_ILNUM native-variable contract introduced in dotnet#128479 instead of the obsolete CALL_INSTRUCTION source-map encoding. The interpreter now emits one ICorDebugInfo::CALL_RETURN_ILNUM NativeVarInfo entry per value-returning managed call site through the standard setVars path. Each entry describes the call's destination var (an FP-relative stack slot) as the return value home, with callReturnValueILOffset set to the call's IL offset and startOffset set to the post-call native offset. The generic DI consumer (GetReturnValueVariableHomes / GetNativeVariable) handles the VLT_STK location identically to ordinary interpreter locals, so all of the PR's former DI-side instruction-decoding additions are obsolete and dropped (reverted to main): module.cpp, rsthread.cpp, rspriv.h, dacdbiimpl.cpp, and the dacdbistructures isInterpreted plumbing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The debugger resolves managed return values only for IL call/callvirt opcodes (CordbNativeCode::GetCallSignature), so CALL_RETURN_ILNUM entries emitted for calli sites are never consumed and only grow the var table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This PR is rebased on #128479, in which EmitCall flags each IL call/callvirt site that returns a value. |
Description
This PR adds managed return value support for interpreted methods so the debugger can surface the value a method just returned when stepping over or out of a call. Building on the variable-home contract from #128479 where
EmitCallflags each value-returning ILcall/callvirtsite. The interpreter stores every return value in that slot, so the generic DI consumer reads it like any other variable home with no interpreter-specific disassembly or ABI logic.Testing
Ran the MRV debugger tests against a local Checked CoreCLR build with
DOTNET_InterpMode=3and all active MRV tests pass under the interpreter: